fix double experiment directory creation during training#46
fix double experiment directory creation during training#46maedmatt wants to merge 3 commits intoamazon-far:mainfrom
Conversation
|
Hey @maedmatt can you try and rebase this? We've updated the static check CI test to output the diff after pre-commit has been run. Perhaps the version of ruff you ran differs from the version ran in CI. |
| if tyro_config.experiment_dir is not None: | ||
| experiment_dir = Path(tyro_config.experiment_dir) | ||
| else: | ||
| from holosoma.utils.experiment_paths import get_experiment_dir, get_timestamp # noqa: PLC0415 |
There was a problem hiding this comment.
Could you import this at the top of the file? Looks like we didn't before, but unsure why we wouldn't.
There was a problem hiding this comment.
I've moved the top-level import in 34c243a
maedmatt
force-pushed
the
fix/double-experiment-dir
branch
from
dfa11ca to
34c243a
Compare
|
Hey @clayrosenthal, I've rebased into main and moved the import to the top of the file. Let me know if I have to adjust something else! |
|
@maedmatt Still seems to be failing the pre-commit ruff checks, have you installed and ran the pre-commit hooks locally? I just merged a PR to try and display the diff for the auto-fix, but you may need to rebase once more to see that If you have already installed the pre-commit hooks, which version of |
base_task.py was calling get_timestamp() independently, creating a second experiment directory a few seconds after train_agent.py. Now train_agent.py passes its experiment_dir through EnvConfig so both use the same directory.
maedmatt
force-pushed
the
fix/double-experiment-dir
branch
from
34c243a to
766fc16
Compare
|
Hey @clayrosenthal, I've rebased it again, let me know if it works now! This is what I have: python 3.8.20, pre-commit 3.5.0, ruff v0.11.8. But isn't the version fixed by |
2 participants
Issue
Issue is described in #44
Fix
Pass the
experiment_dircomputed intrain_agent.pythroughEnvConfigtobase_task.pyinstead of havingbase_task.pycompute its own. Added an optionalexperiment_dir: str | None = Nonefield toEnvConfig. Other callers (replay, eval, tests) that don't pass it get the existing fallback behavior.Test plan
ruff checkpassesmypypasses (no new issues)python -m holosoma.train_agent exp:g1-23dof --algo.config.num_learning_iterations=5creates exactly one directoryBy submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.